feat(credentials): store and processing generic app credentials #1466

s1fr0 · 2022-12-15T00:47:46Z

This PR adds a dynamic keystore for membership credential storage and management. The credentials are thought to be generic membership credentials, i.e. associated to a membership contract and membership Merkle tree, and the implementation is not tied to RLN credentials only.

The keystore JSON schema implemented is the one proposed in #1238 (comment).

In short, a keystore is uniquely identified by an application name, app identifier and version strings. Multiple keystores can co-exist in the same file.
Each keystore can contain none or more Membership credentials. Each Membership Credential contains a unique Identity Credential (made of identity trapdoor, nullifier, secret hash and commitment) and none or more Membership Groups. Each Membership Group contains a Membership contract and a membership tree index position. A Membership Contract contains the chain ID and the membership contract address information.

When a Membership credential is added to a certain keystore:

if its Identity Credential was present in the keystore (it decrypts with the same password used to add the new credential), only the new Membership Groups (i.e. not already present in the keystore) will be added;
if not present, the whole credential is added.
When one or more credentials are added, the keystore is automatically updated to disk (with a backup mechanism to mitigate credentials loss in case of IO errors).

Membership credentials in a certain keystore can be retrieved by none or more filters. These allow to return:

all Membership credentials decrypting under a certain password;
Membership credentials corresponding to one or more Identity credentials;
Membership credentials corresponding to one or more Membership Groups (non-matching groups are stripped);
Membership credentials corresponding to one or more Identity credentials and Membership Groups (non-matching groups are stripped).

This PR addresses the issues:

status-im-auto · 2022-12-15T23:17:25Z

Jenkins Builds

Click to see older builds (7)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`b6466f6`	#1	2022-12-15 23:17:24	~30 min	`macos`	📦`bin`

✔️	`287b10a`	#3	2023-01-11 22:58:16	~10 min	`macos`	📦`bin`

❌	`cc75811`	#4	2023-01-23 22:55:30	~8 min	`macos`	📄`log`
❌	`cc75811`	#1	2023-01-24 09:55:38	~6 min	`linux`	📄`log`

❌	`7566cad`	#5	2023-01-24 22:53:38	~6 min	`macos`	📄`log`

❌	`58724d7`	#6	2023-01-25 23:13:26	~26 min	`macos`	📄`log`

✔️	`1acb693`	#7	2023-01-27 23:01:37	~14 min	`macos`	📦`bin`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`1fcf016`	#8	2023-01-31 23:04:04	~16 min	`macos`	📄`log`

✔️	`af90ba5`	#9	2023-02-06 15:20:38	~33 min	`macos`	📦`bin`

jm-clius · 2023-01-12T10:54:51Z

@s1fr0 @rymnc I notice this draft PR has a release milestone attached - should we remove the milestone (not required for PRs, but rather for issues), or bump it to a next milestone?

s1fr0 · 2023-01-12T19:11:41Z

Thanks @Hanno! We moved delivery of this PR to next release, hence I bumped it.

… only new membership groups

…l, filter

… credentials

rymnc

Few queries, looks great!

Maybe we should decide if we plan to merge this, or #1496 first. There would be non-trivial refactoring both ways.

waku/v2/node/waku_node.nim

waku/v2/utils/credentials.nim

LNSD · 2023-01-31T13:25:32Z

Wow! +696 new lines! 😨 I will need a couple of days to review this.

Please, let's keep the PR sizes manageable. Smaller PRs will help us be more agile in reviewing and merging code changes.

rymnc

LGTM, maybe we should have instructions for operators to use this feature :)

LNSD

I find the code a little convoluted. Please, check my comments.

LNSD · 2023-02-06T14:57:29Z

waku/v2/utils/credentials.nim

@@ -0,0 +1,401 @@
+when (NimMajor, NimMinor) < (1, 4):


I am against adding more stuff under the utils module.

Please, move the utils/keyfile and utils/credentials modules under the waku_rln_relay module given that at this moment it is the only user of this code.

Agreed to move all keyfile/keystore implementation under a new waku_keystore module in protocol. Done in 082043c

LNSD · 2023-02-06T15:05:58Z

waku/v2/utils/credentials.nim

+type AppKeystore* = object
+  application*: string
+  appIdentifier*: string
+  credentials*: seq[MembershipCredentials]
+  version*: string
+
+type
+  AppKeystoreError = enum
+    OsError               = "keystore error: OS specific error"
+    IoError               = "keystore error: IO specific error"
+    JsonKeyError          = "keystore error: fields not present in JSON"
+    JsonError             = "keystore error: JSON encoder/decoder error"
+    KeystoreDoesNotExist  = "keystore error: file does not exist"
+    CreateKeystoreError   = "Error while creating application keystore"
+    LoadKeystoreError     = "Error while loading application keystore"
+    CreateKeyfileError    = "Error while creating keyfile for credentials"
+    SaveKeyfileError      = "Error while saving keyfile for credentials"
+    ReadKeyfileError      = "Error while reading keyfile for credentials"


Could you separate the AppKeystore related code into another module and import and use it here?

Done in 082043c

LNSD · 2023-02-06T15:06:46Z

waku/v2/utils/credentials.nim

+# Encodes a Membership credential to a byte sequence
+proc encode*(credential: MembershipCredentials): seq[byte] =
+  # TODO: use custom encoding, avoid wordy json
+  var stringCredential: string
+  # NOTE: toUgly appends to the string, doesn't replace its contents
+  stringCredential.toUgly(%credential)
+  return toBytes(stringCredential)
+
+# Decodes a byte sequence to a Membership credential
+proc decode*(encodedCredential: seq[byte]): KeystoreResult[MembershipCredentials] =
+  # TODO: use custom decoding, avoid wordy json
+  try:
+    # we parse the json decrypted keystoreCredential
+    let jsonObject = parseJson(string.fromBytes(encodedCredential))
+    return ok(to(jsonObject, MembershipCredentials))
+  except JsonParsingError:
+    return err(JsonError)
+  except Exception: #parseJson raises Exception
+    return err(OsError)
+
+# Checks if a JsonNode has all keys contained in "keys"
+proc hasKeys*(data: JsonNode, keys: openArray[string]): bool =
+  return all(keys, proc (key: string): bool = return data.hasKey(key))


All the JSON serialization-related code should be in a separate module. Is that possible?

Sure. Separated encoding/decoding of keystore relevant data structures from more general "utils". Done in 082043c

…s reviewers

LNSD

Thanks for adding it inside the waku_keystore. I like the new module layout.

There are minor/nitpick details, but in general, LGreatTM ✅

rymnc added this to the Release 0.14.0 milestone Dec 15, 2022

rymnc added the track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications label Dec 15, 2022

s1fr0 modified the milestones: Release 0.14.0, Release 0.15.0 Jan 12, 2023

rymnc added track:rlnp2p and removed track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications labels Jan 19, 2023

s1fr0 added 4 commits January 23, 2023 16:46

feat(credentials): store and processing generic app credentials

cc64ac8

feat(credentials): separate module; minimal tests

49faf2b

more work

bb9d869

feat(credentials): check presence of idCredential in keystore and add…

cc75811

… only new membership groups

s1fr0 force-pushed the app-credentials branch from 287b10a to cc75811 Compare January 23, 2023 15:47

s1fr0 added 10 commits January 24, 2023 13:30

feat(credential): refactor, new data structure, dynamic add credentia…

d1afdeb

…l, filter

feat(credential): add filter, get credentials

7566cad

feat(credential): encode/decode utility

48fc72c

feat(credential): sort groups, test credential retrieval/group merging

58724d7

fix(credential): remove unnecessary order in sort

03cae54

Merge branch 'master' into app-credentials

32ac620

fix(credentials): fix vendor commits

d133e5d

fix(credential/rln): embed credential module in rln relay

1acb693

Merge branch 'master' into app-credentials

dda0591

feat(credentials/rln): use credentials API in rln-relay to store/read…

1fcf016

… credentials

s1fr0 requested review from rymnc and LNSD January 31, 2023 01:50

s1fr0 marked this pull request as ready for review January 31, 2023 01:50

rymnc reviewed Jan 31, 2023

View reviewed changes

refactor(credentials): implement hasKeys for JsonNode

caa4a33

s1fr0 added 5 commits February 6, 2023 00:20

fix(credentials): restore connectToNodes call

a135568

refactor(credentials): remove unnecessary imports

8e96bce

refactor(credentials): add Res suffix to results

1581e70

refactor(credential): moved save json to separate proc; added comments

b57c6ba

feat(credentials): use appInfo

af90ba5

s1fr0 requested a review from rymnc February 6, 2023 00:32

rymnc approved these changes Feb 6, 2023

View reviewed changes

LNSD suggested changes Feb 6, 2023

View reviewed changes

s1fr0 added 2 commits February 7, 2023 16:45

refactor(keystore): refactor code in a more structured module; addres…

082043c

…s reviewers

Merge branch 'master' into app-credentials

2a44956

s1fr0 requested a review from LNSD February 7, 2023 15:49

fix(keystore): fix indentation

ae0a117

LNSD approved these changes Feb 8, 2023

View reviewed changes

s1fr0 merged commit cdc09ae into master Feb 8, 2023

s1fr0 deleted the app-credentials branch February 8, 2023 15:26

rymnc mentioned this pull request Feb 28, 2023

feat: Encrypting the content of the rln credential file #1114

Closed

1 task

richard-ramos mentioned this pull request May 5, 2023

feat: use identity credentials, and expose hash, bulk insert and delete members functions waku-org/js-rln#58

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(credentials): store and processing generic app credentials #1466

feat(credentials): store and processing generic app credentials #1466

s1fr0 commented Dec 15, 2022 •

edited

Loading

status-im-auto commented Dec 15, 2022 •

edited

Loading

jm-clius commented Jan 12, 2023

s1fr0 commented Jan 12, 2023

rymnc left a comment

LNSD commented Jan 31, 2023 •

edited

Loading

rymnc left a comment

LNSD left a comment

LNSD Feb 6, 2023

s1fr0 Feb 7, 2023

LNSD Feb 6, 2023 •

edited

Loading

s1fr0 Feb 7, 2023

LNSD Feb 6, 2023 •

edited

Loading

s1fr0 Feb 7, 2023

LNSD left a comment

feat(credentials): store and processing generic app credentials #1466

feat(credentials): store and processing generic app credentials #1466

Conversation

s1fr0 commented Dec 15, 2022 • edited Loading

status-im-auto commented Dec 15, 2022 • edited Loading

Jenkins Builds

jm-clius commented Jan 12, 2023

s1fr0 commented Jan 12, 2023

rymnc left a comment

Choose a reason for hiding this comment

LNSD commented Jan 31, 2023 • edited Loading

rymnc left a comment

Choose a reason for hiding this comment

LNSD left a comment

Choose a reason for hiding this comment

LNSD Feb 6, 2023

Choose a reason for hiding this comment

s1fr0 Feb 7, 2023

Choose a reason for hiding this comment

LNSD Feb 6, 2023 • edited Loading

Choose a reason for hiding this comment

s1fr0 Feb 7, 2023

Choose a reason for hiding this comment

LNSD Feb 6, 2023 • edited Loading

Choose a reason for hiding this comment

s1fr0 Feb 7, 2023

Choose a reason for hiding this comment

LNSD left a comment

Choose a reason for hiding this comment

s1fr0 commented Dec 15, 2022 •

edited

Loading

status-im-auto commented Dec 15, 2022 •

edited

Loading

LNSD commented Jan 31, 2023 •

edited

Loading

LNSD Feb 6, 2023 •

edited

Loading

LNSD Feb 6, 2023 •

edited

Loading